-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: emily get transactions by input address #1294
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm, we might need to find a new way here, since I just realized that we do not have the input address/scriptPubKey in the transaction. I'm not sure anything can be done without reaching out to bitcoin-core.
let input_address = { | ||
let params = if is_mainnet { | ||
Params::MAINNET | ||
} else { | ||
Params::REGTEST | ||
}; | ||
Address::from_script(&txin.script_sig, params.clone()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure that we want to use Address here instead of the scriptPubKey
? I feel like we should shy away from addresses if possible and just use the scriptPubKey
. That's what is in the transaction (when it is an output) so it seems most natural to use that and encode it as hex.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, sorry for looking at this too late, but we might not be able to use either the scriptPubKey
or the "address" because we do not have them (they're never included in the transaction when they're being spent). This scriptSig
is not the same as the scriptPubKey
from the transaction output, it's basically an unlocking script that, when paired with the actual scriptPubKey
, allows you to spend the funds. It's often empty too. For example, all deposit request outputs and all signer UTXOs have empty scriptSig
s when spent in a sweep transaction. It's basically only used for legacy scriptPubKey
s like P2PKH. I think that the only option is to abandon the endeavor or use bitcoin core for the information.
Also, since a transaction's first input does not necessarily map to a "definitive" source address for the deposit, we may need to make some other changes here too. For example, we might want to check all input scriptPubKey
s and using that if they all match. Or maybe map all input scriptPubKey
s to the same deposit. All of this is assuming we can get the scriptPubKey
at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Man this is very appreciated! A very simple alternative would be to add an address_hex field in the create_deposit request itself. I am discussing it with @hozzjss and @setbern
No problem 😄.
One thing to consider is how to verify that information if we include it. I couldn't think of a way without having the input transactions or bitcoin core (or a trusted equivalent).
Description
Closes: #1218
This PR builds on top of #1197. Most of the changes are autogenerated code.
Changes
https://{base_url}/deposit/reclaim-pubkey/{pubkey}
endpoint. Follows the same approach as feat: Query By Recipient #1125 adding a new secondary global index to DynamoDB.Testing Information
Checklist: